Skip to content

refactor: Use particle outcome for alive check#5300

Merged
kodiakhq[bot] merged 11 commits intoacts-project:mainfrom
andiwand:fatras-explicit-kill-particle
Apr 4, 2026
Merged

refactor: Use particle outcome for alive check#5300
kodiakhq[bot] merged 11 commits intoacts-project:mainfrom
andiwand:fatras-explicit-kill-particle

Conversation

@andiwand
Copy link
Copy Markdown
Contributor

@andiwand andiwand commented Mar 30, 2026

  • Add explicit Particle::killParticle function
  • Particle::isAlive checks outcome and not absolute momentum
  • Use Acts::fastHypot over std::hypot and deduplcate energy() calculation
  • Rework correctEnergy
    • renamed to loseEnergy and flip the sign of the input
    • check if particle gets stopped and if that is expected; throw if not
    • set outcome to user provided value if stopped

--- END COMMIT MESSAGE ---

@andiwand andiwand added this to the next milestone Mar 30, 2026
@github-actions github-actions bot added Component - Fatras Affects the Fatras module Event Data Model labels Mar 30, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 30, 2026 •
edited
Loading

📊: Physics performance monitoring for ff4818f

Full contents

physmon summary

@github-actions github-actions bot added Infrastructure Changes to build tools, continous integration, ... Changes Performance labels Apr 1, 2026
@andiwand andiwand marked this pull request as ready for review April 2, 2026 06:43
AJPfleger
AJPfleger reviewed Apr 2, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@AJPfleger AJPfleger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not totally clear to me, why the performance changes.

@andiwand
Copy link
Copy Markdown
Contributor Author

andiwand commented Apr 2, 2026

It is not totally clear to me, why the performance changes.

The performance change is also not obvious to me but since we cannot claim it was understood to such degree before I simply went for updating the references. We can definitely check that but it requires a bit more attention and time

AJPfleger
AJPfleger reviewed Apr 2, 2026
View reviewed changes
@asalzburger
Copy link
Copy Markdown
Contributor

asalzburger commented Apr 4, 2026

From my point of view, that's ok to go - it's not that we had this fully debugged before.

E.g. I ran into DividedByZero issues in the schema before when setting the energy to zero to indicate that the particle is not alive anymore, so I would suggest we move that forward.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 4, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@kodiakhq kodiakhq bot merged commit db9a903 into acts-project:main Apr 4, 2026
44 checks passed
@andiwand andiwand deleted the fatras-explicit-kill-particle branch April 4, 2026 07:53
@github-actions github-actions bot removed the automerge label Apr 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AJPfleger AJPfleger AJPfleger left review comments

@asalzburger asalzburger asalzburger approved these changes

Assignees

No one assigned

Labels

Changes Performance Component - Fatras Affects the Fatras module Event Data Model Infrastructure Changes to build tools, continous integration, ...

Projects

None yet

Milestone

next

Development

Successfully merging this pull request may close these issues.

3 participants

@andiwand @asalzburger @AJPfleger